✨ Add regression test cases for DeploymentConfig options#2493
✨ Add regression test cases for DeploymentConfig options#2493openshift-merge-bot[bot] merged 1 commit intooperator-framework:mainfrom
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2493 +/- ##
==========================================
- Coverage 73.68% 68.70% -4.99%
==========================================
Files 102 102
Lines 8471 8601 +130
==========================================
- Hits 6242 5909 -333
- Misses 1749 2220 +471
+ Partials 480 472 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
anik120
left a comment
anik120
left a comment
There was a problem hiding this comment.
🎉 thanks for the PR!
I wonder if we loose anything if we just combine all of the options into one?
This is a lot of test manifests we'll have to store in the repository if we break them down individually.
From POV it's sufficient to include one test with ALL of the configuration options, and list in a comment what the individual options are.
|
Yeah that's totally fair. I just was following the objectives as laid out in the OPRUN ticket but I had wondered the same thing, that just the combined case is really needed. |
Updates generate-manifests.go to include a test case for all option types in DeploymentConfig. Updates the testdata/expected-manifests/ directory with all the new test case manifests. Signed-off-by: Tayler Geiger <tayler@redhat.com>
anik120
left a comment
anik120
left a comment
There was a problem hiding this comment.
Thanks for that update @trgeiger.
Now that I can see things more clearly, I'm wondering if we need to introduce a new bundle for this test. Can't we just the existing argocd-operator.v0.6.0 bundle
|
/approve |
openshift-ci
bot
added
the
approved
rashmigottipati
left a comment
rashmigottipati
left a comment
There was a problem hiding this comment.
Changes look good pending addressing of Anik's comment: #2493 (review)
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: perdasilva, rashmigottipati The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
This already is using the existing bundle, no? It's just adding the options to a separate test case inside that bundle, like the own-namespace, single-namespace, all-namespace cases. |
|
/override codecov/project Otherwise, it will not get merged :-) |
|
@camilamacedo86: Overrode contexts on behalf of camilamacedo86: codecov/project DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
openshift-merge-bot
bot
merged commit 0655ce1
into
operator-framework:main
5 participants
Description
Updates
generate-manifests.goto include test cases for each option type in DeploymentConfig as well as a test case with all DeploymentConfig options.Updates the
testdata/expected-manifests/directory with all the new test case manifests.Large number of file changes is entirely due to all the new manifests for each test case in
testdata/expected-manifests/. Review should mainly focus ongenerate-manifests.gochanges.Reviewer Checklist